Skip to content

Use config server for undulator config#1768

Open
jacob720 wants to merge 4 commits intomainfrom
mx_bluesky_1494_use_config_server_for_undulator
Open

Use config server for undulator config#1768
jacob720 wants to merge 4 commits intomainfrom
mx_bluesky_1494_use_config_server_for_undulator

Conversation

@jacob720
Copy link
Contributor

@jacob720 jacob720 commented Dec 8, 2025

Fixes DiamondLightSource/mx-bluesky#1494

Required by DiamondLightSource/mx-bluesky#1497

Instructions to reviewer on how to test:

  1. Confirm config is read through the config server

Checks for reviewer

  • Would the PR title make sense to a scientist on a set of release notes
  • If a new device has been added does it follow the standards
  • If changing the API for a pre-existing device, ensure that any beamlines using this device have updated their Bluesky plans accordingly
  • Have the connection tests for the relevant beamline(s) been run via dodal connect ${BEAMLINE}

@jacob720 jacob720 requested a review from a team as a code owner December 8, 2025 14:56
@jacob720 jacob720 marked this pull request as draft December 8, 2025 14:56
@jacob720 jacob720 force-pushed the mx_bluesky_1494_use_config_server_for_undulator branch from 20ccd5b to 3df99f3 Compare December 9, 2025 08:34
@jacob720 jacob720 changed the title Mx bluesky 1494 use config server for undulator Use config server for undulator config Jan 6, 2026
@jacob720 jacob720 force-pushed the mx_bluesky_1494_use_config_server_for_undulator branch from 2cc6d15 to f7c7993 Compare January 12, 2026 12:31
@codecov
Copy link

codecov bot commented Jan 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.14%. Comparing base (e0d5a17) to head (f7c7993).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1768   +/-   ##
=======================================
  Coverage   99.14%   99.14%           
=======================================
  Files         288      288           
  Lines       10937    10939    +2     
=======================================
+ Hits        10843    10845    +2     
  Misses         94       94           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jacob720 jacob720 marked this pull request as ready for review January 12, 2026 13:26
Comment on lines +189 to 190
self.config_server = ConfigServer(url="https://daq-config.diamond.ac.uk")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if this hardcoded config server is a good solution long term. I understand that it works fine now, but what if for some reason you need to divert your config server to a different endpoint - you would have to change the path here.
It feels like we need some king of an interface or a lookuptable provider which should probably be configured separately.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Snap! I think my comment at #1773 (comment) is basically the same thing. I think we should define it in the ixx.py files like we do for e.g. a path provider

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, agreed - in an even broader perspective I don't think we should need daq-config-server as a dependency - but that's for later development DiamondLightSource/daq-config-server#157

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate Undulator and UndulatorDCM config to use the daq config server

3 participants

Comments